feat: consolidate codegen test helper + runtime sub-export#1023
Open
pyramation wants to merge 8 commits intomainfrom
Open
feat: consolidate codegen test helper + runtime sub-export#1023pyramation wants to merge 8 commits intomainfrom
pyramation wants to merge 8 commits intomainfrom
Conversation
#1: Move runCodegenAndLoad into @constructive-io/graphql-test - New codegen-helper.ts in graphql-test/src/ with full codegen pipeline - Supports both positional (GraphQLQueryFn) and object-style (GraphQLQueryFnObj) - Re-exports getDbConnections and types from pgsql-test for two-phase patterns - orm-test now imports from @constructive-io/graphql-test instead of local copy - Deleted local orm-test codegen-helper.ts #2: Add runtime sub-export to @constructive-io/graphql-query - New graphql-query/src/runtime/index.ts re-exports: - parseType, print from @0no-co/graphql.web - All gql-ast exports - GraphQLAdapter, GraphQLError, QueryResult types from graphql-types - Updated codegen templates (query-builder.ts, orm-client.ts) to import from @constructive-io/graphql-query/runtime instead of 3 separate packages - Updated codegen test snapshots and assertions to match new import paths
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
The runtime sub-export re-exports parseType, print, and type-only symbols. Using 'import * as t from runtime' would pollute the t namespace with unrelated symbols. Keep 'import * as t from gql-ast' as-is in templates.
…tion The cli-e2e tests generate ORM code in /tmp and require the runtime sub-path. Without an explicit exports map, Node cannot resolve @constructive-io/graphql-query/runtime. This adds exports for both the root and ./runtime sub-paths (CJS, ESM, and types).
…lution The cli-e2e tests spawn child processes in /tmp that need to resolve @constructive-io/graphql-query/runtime. Adding the package to resolveNodePaths() so its node_modules path is included in NODE_PATH.
…b-path resolution The cli-e2e tests spawn child processes that need to resolve @constructive-io/graphql-query/runtime via NODE_PATH. Adding the package as a devDependency so pnpm creates a symlink in server-test/node_modules, making the exports field accessible.
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
Add exports field to graphql-codegen package.json with ./orm sub-path so generateOrm is a stable contract instead of a filesystem crawl. Update codegen-helper.ts to use clean import from @constructive-io/graphql-codegen/orm.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three packaging consolidations that emerged from the PR #934 refactoring work:
#1 — Move
runCodegenAndLoadinto@constructive-io/graphql-testThe codegen test helper (introspect → infer tables → generate ORM → compile TS → load
createClient) was duplicated betweenorm-testandconstructive-db. This PR:graphql/test/src/codegen-helper.tswith a version that supports both positional (GraphQLQueryFn) and object-style (GraphQLQueryFnObj) query signaturesgetDbConnectionsand related types frompgsql-testfor two-phase test patterns@constructive-io/graphql-testorm-test/__tests__/helpers/codegen-helper.ts#2 — Add
@constructive-io/graphql-query/runtimesub-exportGenerated ORM code currently imports from 3 separate runtime packages (
@0no-co/graphql.web,gql-ast,@constructive-io/graphql-types). This PR consolidates two of the three:graphql/query/src/runtime/index.tsthat re-exports@0no-co/graphql.web(parseType,print) and@constructive-io/graphql-types(GraphQLAdapter,GraphQLError,QueryResult)exportsfield tographql-query/package.jsonmapping./runtimetoruntime/index.js(CJS) andesm/runtime/index.js(ESM)query-builder.tstemplate:parseType/printnow import from@constructive-io/graphql-query/runtimeorm-client.tstemplate:GraphQLAdapter/GraphQLError/QueryResulttypes now import from@constructive-io/graphql-query/runtimegql-astis intentionally not re-exported —import * as t from 'gql-ast'stays as-is to avoid polluting thetnamespace with unrelated symbols likeparseTypeandprintclient-generator.test.ts#3 — Add
@constructive-io/graphql-codegen/ormand./clisub-exportsThe codegen-helper previously used a fragile
require.resolve+path.joinhack to reach into@constructive-io/graphql-codegeninternals forgenerateOrm. Thecli-e2e.test.tssimilarly used deep path imports (@constructive-io/graphql-codegen/core/codegen/cli). This PR:exportsfield tographql-codegen/package.jsonwith./ormand./clisub-paths mapping tocore/codegen/{orm,cli}/index.js(CJS) andesm/core/codegen/{orm,cli}/index.js(ESM)typesVersionstographql-codegen/package.json— required because the roottsconfig.jsonusesmoduleResolution: "node"which doesn't support theexportsfield. Packages likegraphql/server-testthat inherit this setting needtypesVersionsfor TypeScript to resolve the sub-path types correctly. (graphql/testoverrides to"nodenext"so it works without this.)require.resolvehack incodegen-helper.tswith a cleanimport { generateOrm } from '@constructive-io/graphql-codegen/orm'cli-e2e.test.tsto import from@constructive-io/graphql-codegen/cliand@constructive-io/graphql-codegen/orminstead of deep internal pathsUpdates since last revision
Fixed
cli-e2e.test.tsfailures — the server-test suite spawns child processes in/tmp/withNODE_PATHto resolve runtime modules. Two changes were needed:@constructive-io/graphql-queryas a devDependency ofgraphql-server-test— pnpm workspace isolation means the package isn't resolvable unless explicitly declared.@constructive-io/graphql-queryto theruntimeDepsarray inresolveNodePaths()— so the function collects the correctnode_modulesdirectories for the child process.Replaced
require.resolvehack with./ormsub-path export — addedexportsfield tographql-codegen/package.jsonand updatedcodegen-helper.tsto use a clean ESM-safe import instead of filesystem crawling.Added
./clisub-path export —cli-e2e.test.tswas importing from@constructive-io/graphql-codegen/core/codegen/cli, which breaks once theexportsfield is present (it restricts resolvable paths). Added./clito the exports and updated the test imports.Added
typesVersionsformoduleResolution: "node"compat — the roottsconfig.jsonusesmoduleResolution: "node"which ignores theexportsfield for type resolution.graphql/server-testinherits this setting. WithouttypesVersions, TypeScript couldn't resolve types for@constructive-io/graphql-codegen/ormor./cli. Note:graphql/testalready overrides tomoduleResolution: "nodenext", so it didn't need this.Review & Testing Checklist for Human
typesVersionsvsbinfor./cli: The package has"bin": { "graphql-codegen": "cli/index.js" }(the CLI binary atdist/cli/) and"exports": { "./cli": ... }pointing todist/core/codegen/cli/(the generator code). WithmoduleResolution: "node", TypeScript would normally resolve@pkg/clitodist/cli/index.d.ts(the binary). ThetypesVersionsoverrides this to the correct generator types. Verify this doesn't cause issues for any consumer that might import from thecli/binary path directly../ormand./clisub-paths expose full internal APIs as public.orm/index.tsexportsgenerateOrmplus all sub-generators (generateAllModelFiles,generateOrmClientFile, etc.).cli/index.tsexportsgenerateCli,generateMultiTargetCli, and all CLI generators. Previously these were internal. Confirm this surface area is acceptable.query-builder.tsandorm-client.tsnow import from@constructive-io/graphql-query/runtime. All downstream repos will produce new import paths on next codegen run. Verify they already have@constructive-io/graphql-queryas a dependency and that the/runtimesub-path resolves.codegen-helper.ts:queryFn.length <= 1to distinguish object-style from positional. Verify this heuristic is correct for all graphile-test query wrappers.orm-testandserver-testsuites against a real DB to confirm the sub-path imports resolve correctly end-to-end (CI covers this, but worth watching).Notes
gql-astremains a direct import inquery-builder.ts(import * as t from 'gql-ast') — this is intentional to keep thetnamespace clean. The runtime sub-export consolidates the other two runtime deps only.pnpm-lock.yamldiff is mostly formatting noise from a pnpm version change — the actual dependency changes are just the added workspace deps.graphql-query/package.jsondoes not needtypesVersionscurrently because no consumer usingmoduleResolution: "node"imports from./runtime. If that changes, it will need the same treatment.Link to Devin session: https://app.devin.ai/sessions/18879be982854a40abe5c9b915aa4a84
Requested by: @pyramation